Skip to content

Add prw2 pool #6917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Oct 23, 2025
Merged

Add prw2 pool #6917

merged 11 commits into from
Oct 23, 2025

Conversation

@SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Jul 28, 2025 •
edited
Loading

Followed up #6330. This PR adds a PRW2.0 proto and sync.pool to PRW2 to avoid lots of GC.
The #6330 PR needs to be merged first.

These are the results of the benchmark.

benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/cortexproject/cortex/pkg/util/push
cpu: Apple M1 Max
                                     │   old.txt   │               new.txt                │
                                     │   sec/op    │    sec/op     vs base                │
_convertV2RequestToV1/100_series-10    50.97µ ± 1%   23.22µ ±  3%  -54.44% (p=0.000 n=10)
_convertV2RequestToV1/500_series-10    249.2µ ± 0%   111.4µ ±  3%  -55.28% (p=0.000 n=10)
_convertV2RequestToV1/1000_series-10   495.9µ ± 0%   225.4µ ± 14%  -54.56% (p=0.000 n=10)
geomean                                184.7µ        83.54µ        -54.76%

                                     │   old.txt    │               new.txt                │
                                     │     B/op     │     B/op      vs base                │
_convertV2RequestToV1/100_series-10    94.09Ki ± 0%   42.52Ki ± 0%  -54.80% (p=0.000 n=10)
_convertV2RequestToV1/500_series-10    466.7Ki ± 0%   208.9Ki ± 0%  -55.24% (p=0.000 n=10)
_convertV2RequestToV1/1000_series-10   931.8Ki ± 0%   416.2Ki ± 0%  -55.33% (p=0.000 n=10)
geomean                                344.6Ki        154.6Ki       -55.13%

                                     │   old.txt    │               new.txt               │
                                     │  allocs/op   │  allocs/op   vs base                │
_convertV2RequestToV1/100_series-10     1513.0 ± 0%    513.0 ± 0%  -66.09% (p=0.000 n=10)
_convertV2RequestToV1/500_series-10     7.515k ± 0%   2.515k ± 0%  -66.53% (p=0.000 n=10)
_convertV2RequestToV1/1000_series-10   15.016k ± 0%   5.016k ± 0%  -66.60% (p=0.000 n=10)
geomean                                 5.548k        1.864k       -66.41%

goos: darwin
goarch: arm64
pkg: github.com/cortexproject/cortex/pkg/cortexpb
cpu: Apple M1 Max
BenchmarkMarshallWriteRequestV2
BenchmarkMarshallWriteRequestV2/no-pool
BenchmarkMarshallWriteRequestV2/no-pool-10         	  135732	      8818 ns/op	   11152 B/op	     102 allocs/op
BenchmarkMarshallWriteRequestV2/byte_pool
BenchmarkMarshallWriteRequestV2/byte_pool-10       	  282535	      4106 ns/op	    3116 B/op	       2 allocs/op
BenchmarkMarshallWriteRequestV2/byte_and_write_pool
BenchmarkMarshallWriteRequestV2/byte_and_write_pool-10         	  293763	      3961 ns/op	    2772 B/op	       1 allocs/op
PASS

Which issue(s) this PR fixes:
Fixes #6324

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@SungJin1212 SungJin1212 marked this pull request as draft July 28, 2025 05:23
@dosubot dosubot bot added go Pull requests that update Go code type/performance labels Jul 28, 2025
@SungJin1212 SungJin1212 mentioned this pull request Jul 23, 2025
Closed
8 tasks
@SungJin1212 SungJin1212 force-pushed the Add-prw2-pool branch 2 times, most recently from 61c9b79 to ba3259e Compare August 12, 2025 07:23
@SungJin1212 SungJin1212 marked this pull request as ready for review August 12, 2025 09:20
@SungJin1212 SungJin1212 force-pushed the Add-prw2-pool branch 3 times, most recently from 67207fc to 6a538d8 Compare September 17, 2025 05:05
yeya24
yeya24 approved these changes Sep 21, 2025
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@yeya24
Copy link
Contributor

yeya24 commented Oct 23, 2025

Do we need to rebase to fix the flaky test?

@SungJin1212
Support remote write v2 by converting request
09a3052 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Change to not break exist behavior
17f45c3 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
rebase from stream connection
0c2e313 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Change to expose header at Distributor side
954af22 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Add prw2 pool
918aa41 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Fix test
96a3722 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Fix lint
7a214a0 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
get rebase
9933269 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Update test
dc96982 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
Add a changelog
5ff82ce 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212
fix lint
1f83cc7 
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@yeya24 yeya24 merged commit c884d66 into cortexproject:master Oct 23, 2025
62 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yeya24 yeya24 yeya24 approved these changes

Assignees

No one assigned

Labels

go Pull requests that update Go code size/XL type/performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Prometheus Remote Write v2 Implementation

2 participants

@SungJin1212 @yeya24